-
Notifications
You must be signed in to change notification settings - Fork 1k
gps: fix panic for gopkgin's implicit v0 #1243
Conversation
I opened #1244 for the AppVeyor failure, it may be a flakey test. I don't seem to be able to restart builds anymore on AppVeyor to test out that theory though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Let's add this as a bug fix in the changelog. |
internal/gps/vcs_source.go
Outdated
if tv.sv.Major() == s.major && !s.unstable { | ||
vlist[k] = v | ||
k++ | ||
} | ||
case branchVersion: | ||
if tv.name == "master" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this binds us to the literal branch name master
, which isn't guaranteed to be right. despite the gopkg.in docs on their site, it's also not actually what they do:
in the event that nothing matches, they basically just pass the data from upstream straight through.
we may be able to do something less complicated than what we have in gitSource.listVersions()
, but we have to rely on the HEAD
value in the output list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching that. They saying master
in the docs, and in the code comments, and in some hard-coded strings so I thought that's what they were using.
201cb87
to
8dc5eb9
Compare
I have updated the logic to use the default branch returned from |
b0cbc62
to
5218e33
Compare
I've rebased for great goodness. |
5218e33
to
d752f83
Compare
When v0 is requested, and there are no semver branches/tags, fallback to the default branch. See http://labix.org/gopkg.in#VersionZero
d752f83
to
e845081
Compare
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the |
What does this do / why do we need it?
When v0 is requested, and there are no semver branches/tags, fallback to the master branch.
See http://labix.org/gopkg.in#VersionZero
What should your reviewer look out for in this PR?
Nope.
Do you need help or clarification on anything?
Nope.
Which issue(s) does this PR fix?
Fixes #933
Fixes #776